Skip to content

Show both checksums per file, beside the queue - #12

Open
owenpkent wants to merge 14 commits into
shell-context-menufrom
file-list-pane
Open

owenpkent wants to merge 14 commits into
shell-context-menufrom
file-list-pane

Conversation

@owenpkent

@owenpkent owenpkent commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

Summary

The interface could say a job was "Verified" without ever showing why. The source checksum and the destination's — the claim this whole tool rests on — lived only in the CSV and the PDF, so the person deciding whether to reformat a card had to open a report to see the evidence.

The selected job's files now sit beside the queue: size, both hashes, and a doubled check for a copy that was written and read back. The queue becomes the rail and this is its detail pane, split rather than tabbed so the proof is visible at the same time as the job it belongs to.

Decisions worth reviewing

  • head:tail, not a prefix (28F7:EA9B). A prefix alone cannot distinguish a matching pair from a near-miss that shares one, which is the only thing these two columns exist for. test_abbreviation_keeps_the_tail asserts it.
  • mismatch when destinations disagree with each other. Every copy comes from a single read, so they cannot legitimately differ — showing the first and hiding the rest would bury a real finding.
  • Green only when it has proved something: the pair agrees and all destinations agree. A file the engine called COPIED whose destination hash differs from the source gets no green.
  • Tooltips carry the full values, every destination path, and any per-destination error. The abbreviation is for scanning; checking against a manifest needs all of it.
  • Rebuild is skipped for the same job object. The panel hears about the selection on every progress event and would otherwise reset the table dozens of times a second.

Most of the 16 tests cover the cases where the claim is not clean, since those are the rows that must not be misread as verified.

On the left rail

The other half of the inspiration was a vertical job rail. I did not build one: this window already has a left rail (drives), and a second would be clutter rather than clarity. Making the queue the rail with a detail pane gets the same structure without it. Say the word if you want actual vertical job cards instead.

Screenshot tool

Two fixes it needed to document this honestly:

  • A finished job with real structure, so the pictures show the pane working rather than inviting the reader to select something.
  • A trailing progress sample. The rate is measured against the clock now, so a hand-built queue item had none and the throughput column rendered blank — a regression the committed images predate. Stamped immediately before each shot, because a sample older than the window reads as no rate at all, which made the figure come and go with however long Qt took to start.

All three README images are regenerated, since the layout they document changed.

Test plan

  • tests/test_gui_file_list.py — 16 tests: abbreviation incl. tail-sensitivity, verified pair coloured, mismatched destination not coloured verified, destinations disagreeing say mismatch, failed file marked, tooltip carries full hashes, summary counts "verified 2 of 3 / 1 failed", no rebuild on repeat, waiting message still updates
  • Full suite 516 passed, 3 skipped; ruff clean (extends the existing Qt B008 per-file ignore already granted to queue_view.py)
  • Screenshots regenerate cleanly; real offloader-gui.exe launches and runs

Review follow-ups (2026-10-12)

The pane could imply a read-back that never happened. Now: green means read back from the destination and matched; a failed page-cache eviction (new Job.cache_eviction_failed) removes the read-back mark; checksum "none" gets a size-only mark and a "size-checked" summary; failed destinations show no digest; a cancelled job says how far it got (new Job.scanned_files); warnings are counted in the header; C4 identifiers keep their case when abbreviated; a mismatch says "mismatch" in red; and the source-only column header reads "As written".

The interface could say a job was "Verified" without ever showing why. The
source checksum and the destination's — the claim the whole tool rests on —
existed only in the CSV and the PDF, so the person deciding whether to
reformat a card had to open a report to see the evidence for it.

The selected job's files now sit next to the queue: size, both hashes, and a
doubled check mark for a copy that was written and read back. The queue is the
rail and this is its detail pane, split rather than tabbed so the proof is
visible at the same time as the job it belongs to.

Hashes are abbreviated head:tail rather than truncated, because a prefix alone
cannot tell a matching pair from a near-miss that happens to share one — which
is the only thing these two columns are for. Full values, every destination
path and any per-destination error are in the tooltip.

The colour is deliberately conservative: green only when the pair agrees and
every destination agrees with the others too. Destinations that disagree read
"mismatch" rather than showing the first and hiding the rest — both copies come
from a single read, so they cannot legitimately differ, and that is a finding
rather than a detail. A file the engine called copied whose destination hash
differs from the source gets no green either.

Rebuilding is skipped when the same job object comes back, since the panel is
told about the selection on every progress event and would otherwise reset the
table dozens of times a second.

The screenshot tool gains a finished job so the pictures show the pane doing
its work rather than inviting the reader to select something, and a trailing
progress sample so the queue's throughput column is populated: the rate is
measured against the clock now, so a hand-built item had none and the column
came out blank.
Only disagreement was covered, so a model that reported 'mismatch' for every multi-destination job would have passed the suite while making the normal two-copy preset look like a fault. The tooltip is pinned to list every destination and any per-destination error, since the row has no room for either, and a file with no destinations at all — a skipped one — is checked not to fall through to the colour that means proved.

@owenpkent owenpkent left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the new file model/pane, job selection wiring, checksum abbreviation, multi-destination aggregation, tooltips, and queue/file split layout.

Validation: 20 targeted file-list tests passed on this head and git diff --check passed. The full checksum and each destination's path/status remain available in the tooltip, and disagreement between populated destination checksums is represented explicitly instead of showing only the first result.

A verification-mode presentation concern is included in the private review notes under the repository's integrity policy. It concerns the evidence implied by the new indicator, not a change to the engine's copied bytes. No interactive desktop walkthrough was performed.

@owenpkent owenpkent left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed c139b06 for checksum presentation, multi-destination aggregation, full-value tooltips, and the evidence implied by the new status indicator.

The earlier review mentioned a verification-mode presentation concern without inline detail. A direct engine-to-model reproduction confirms it: a source-only job gets the double check documented as destination read-back, even though no destination read-back occurred. Details and suggested coverage are inline.

Validation: all 20 existing file-list tests passed under headless Qt, followed by a separate real-copy reproduction using VerificationMode.SOURCE_ONLY. The tests currently construct their display jobs with FULL, so passing them does not exercise that distinction.

Comment thread src/offloader/gui/file_list.py

@owenpkent owenpkent left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No additional actionable defect found in this incremental diff.

Reviewed queue selection wiring, per-file and per-destination checksums, status presentation, tooltip/summary behavior and screenshot fixtures. The earlier SOURCE_ONLY/readback presentation issue is fixed: the UI now consults the job verification mode and qualifies what the displayed digest represents. The relevant engine-produced regression coverage is present.

Validation: 151 focused screenshot/retry/stall/shell/file-list tests and two startup-source tests passed on this exact detached head, with four registry-mutating cases deselected. Three committed screenshots were inspected and use synthetic labels/paths. Current-head CI has successful runs plus older cancelled duplicates; the diff check passes.

Limits: headless Qt, without an interactive large-delivery session or registry round trip. The parent #10 watchdog findings are inherited and reported there, not duplicated on this file-pane change.

Pinned scope: declared base 2c377c7d5e87f7005912e9e488222fb74d8bb804, merge base 2c377c7d5e87f7005912e9e488222fb74d8bb804, head 14c496e5f8f5424541d0458b61914ef734659782. Existing discussion and current-head checks were reviewed; no threads were resolved and no formal approval or merge is being issued.

The pane could imply a read-back that never happened. A cancelled job
summarised as verified N of N; a failed page-cache eviction still showed the
read-back mark; checksum "none" showed two ticks for a size compare; the
destination column turned green in copy-only and source-only modes, where
both hashes come from the same buffer; a failed destination showed its
write-stream digest in green; C4 identifiers were upper-cased when
abbreviated, so two different IDs could look the same; and a mismatch showed
only as a missing colour.

The job now records how many files the scan found and whether eviction
failed, and the pane uses those: green means read back from the destination
and matched, size-only compares get their own mark, failed copies show no
digest, warnings are counted in the header, and a mismatch says so in red.
On macOS CI the whole pytest process died with a segmentation fault inside
an unrelated retry test. Parentless widgets left by the GUI tests were being
garbage-collected on the engine's reader thread, the thread that happened to
allocate, so QWidget destructors ran off the GUI thread. An autouse fixture
now processes events and collects on the main thread after each test once a
QApplication exists.
…sume it

Eviction always fails on macOS, so the full-verification read-back test saw
one tick instead of two and the warning count test saw the eviction warning
as well as the retry it was counting. Both tests now patch eviction to
succeed; the failed-eviction case has its own test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant